Skip to content

Add support for ffmpeg_image_transport to Image/Camera displays and point_cloud_transport/cloudini to PointCloud2 display #1568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 29, 2025
Merged

Add support for ffmpeg_image_transport to Image/Camera displays and point_cloud_transport/cloudini to PointCloud2 display #1568

merged 1 commit into from
Aug 29, 2025

Conversation

lreiher
Copy link
Contributor

@lreiher lreiher commented Aug 26, 2025

Description

  • fix topic handling in Camera display to support images compressed by any supported image_transport
  • make Image and Camera display support images compressed by ffmpeg_image_transport
  • make PointCloud2 display support point clouds compressed by cloudini

Is this user-facing behavior change?

  • users can now select ffmpeg-compressed images in Image and Camera displays
  • users can now select cloudini-compressed point clouds in PointCloud2 displays

Did you use Generative AI?

Yes, GitHub Copilot auto-completion.

Additional Information

Related to #1288?

lreiher
lreiher commented Aug 27, 2025
View reviewed changes
rviz_default_plugins/src/rviz_default_plugins/displays/camera/camera_display.cpp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again test if these changes are necessary and how they interact with the transport selection that Camera display offers when adding a new display.

@lreiher lreiher mentioned this pull request Aug 27, 2025
Draft
@roncapat
Copy link
Contributor

Question (maybe more for the maintainers): why is there the need to hardcode 3rd party formats in the code? Wasn't the image_transport thing meant to be extendable without requiring code modifications?

@lreiher
Copy link
Contributor Author

lreiher commented Aug 28, 2025

I had the exact same question, but as a first step, simply wanted to add the support consistently with the current implementation.

point_cloud_transport (and, most likely, image_transport in the same way) is indeed able to switch between TransportHints at runtime without hardcoding any supported plugins or topic postfixes via the point_cloud_transport parameter (example in tutorials).

@ahcorde
Copy link
Contributor

ahcorde commented Aug 28, 2025

Pulls: #1568
Gist: https://gist.githubusercontent.com/ahcorde/17fba807c5c0d6fe27b48db13d3ad391/raw/5b0b6b9ec6a01842e0e1807f61737ed1b9a71541/ros2.repos
BUILD args: --packages-above-and-dependencies rviz_common
TEST args: --packages-above rviz_common
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16800

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde
Copy link
Contributor

ahcorde commented Aug 29, 2025

@roncapat and @lreiher Happy to review a PR to remove the hardcoded transport hints

@ahcorde ahcorde merged commit a33b5e9 into ros2:rolling Aug 29, 2025
1 of 2 checks passed
@ahcorde
Copy link
Contributor

ahcorde commented Aug 29, 2025

https://github.com/Mergifyio backport kilted jazzy humble

@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 29, 2025 •
edited
Loading

backport kilted jazzy humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Aug 29, 2025
@lreiher @mergify
add support for ffmpeg_image_transport and point_cloud_transport (#1568)
7b9a427 
Co-authored-by: Emre Karincali <[email protected]>
(cherry picked from commit a33b5e9)
mergify bot pushed a commit that referenced this pull request Aug 29, 2025
@lreiher @mergify
add support for ffmpeg_image_transport and point_cloud_transport (#1568)
5835d63 
Co-authored-by: Emre Karincali <[email protected]>
(cherry picked from commit a33b5e9)
mergify bot pushed a commit that referenced this pull request Aug 29, 2025
@lreiher @mergify
add support for ffmpeg_image_transport and point_cloud_transport (#1568)
196d01d 
Co-authored-by: Emre Karincali <[email protected]>
(cherry picked from commit a33b5e9)

# Conflicts:
#	rviz_default_plugins/src/rviz_default_plugins/displays/pointcloud/get_transport_from_topic.cpp
This was referenced Aug 29, 2025
Merged
Merged
Merged
@lreiher lreiher deleted the ffmpeg-image-transport-point-cloud-transport-public branch September 1, 2025 06:34
ahcorde pushed a commit that referenced this pull request Sep 1, 2025
@mergify @lreiher
add support for ffmpeg_image_transport and point_cloud_transport (#1568…
20559ef 
…) (#1570)

(cherry picked from commit a33b5e9)

Co-authored-by: Lennart Reiher <[email protected]>
Co-authored-by: Emre Karincali <[email protected]>
ahcorde pushed a commit that referenced this pull request Sep 1, 2025
@mergify @lreiher
add support for ffmpeg_image_transport and point_cloud_transport (#1568…
b87d701 
…) (#1571)

(cherry picked from commit a33b5e9)

Co-authored-by: Lennart Reiher <[email protected]>
Co-authored-by: Emre Karincali <[email protected]>
ahcorde added a commit that referenced this pull request Sep 1, 2025
@mergify @lreiher @ahcorde
Add support for ffmpeg_image_transport to Image/Camera displays and p…
8b6c336 
…oint_cloud_transport/cloudini to PointCloud2 display (backport #1568) (#1572)

Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Lennart Reiher <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lreiher @roncapat @ahcorde